Skip to content

gh-116738: Make grp module thread-safe#135434

Merged
mpage merged 10 commits intopython:mainfrom
yoney:ft_grpmodule_2
Jul 14, 2025
Merged

gh-116738: Make grp module thread-safe#135434
mpage merged 10 commits intopython:mainfrom
yoney:ft_grpmodule_2

Conversation

@yoney
Copy link
Contributor

@yoney yoney commented Jun 12, 2025 •
edited
Loading

Make grp module methods getgrgid() and getgrnam() thread-safe when the GIL is disabled and getgrgid_r()/getgrnam_r() C APIs are not available.

  • Move run_concurrently() to test.support.threading_helper to resuse across multiple tests.

cc: @mpage @colesbury @Yhg1s

@bedevere-app bedevere-app bot mentioned this pull request Jun 12, 2025
Open
@yoney yoney marked this pull request as ready for review June 12, 2025 16:01
@yoney yoney requested a review from rhettinger as a code owner June 12, 2025 16:01
@yoney
Copy link
Contributor Author

yoney commented Jun 12, 2025

Note: I am not sure if there is a CI setup where getgrgid_r()/getgrnam_r() are not available. I tested those manually on my local machine.

@colesbury colesbury requested review from Yhg1s, colesbury and mpage June 16, 2025 17:15
colesbury
colesbury reviewed Jun 16, 2025
View reviewed changes
@colesbury colesbury added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 16, 2025
@bedevere-bot
Copy link

bedevere-bot commented Jun 16, 2025

🤖 New build scheduled with the buildbot fleet by @colesbury for commit 269b454 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135434%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 16, 2025
@colesbury colesbury added the needs backport to 3.14 bugs and security fixes label Jun 16, 2025
ZeroIntensity
ZeroIntensity reviewed Jun 16, 2025
View reviewed changes
@rhettinger rhettinger removed their request for review June 17, 2025 02:52
@mpage
Copy link
Contributor

mpage commented Jun 26, 2025

@yoney - Can you merge main and fix up the conflicts?

@yoney
Copy link
Contributor Author

yoney commented Jun 26, 2025

@yoney - Can you merge main and fix up the conflicts?

I rebased and force push but I think this is not the right way, I should have merged the main on top of my branch, I am sorry.

@mpage
Copy link
Contributor

mpage commented Jun 26, 2025

I rebased and force push but I think this is not the right way, I should have merged the main on top of my branch, I am sorry.

No worries!

ZeroIntensity
ZeroIntensity reviewed Jun 30, 2025
View reviewed changes
Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core changes look good to me, just a few comments about the new threading_helper function.

kumaraditya303
kumaraditya303 reviewed Jul 3, 2025
View reviewed changes
@yoney yoney requested a review from ericsnowcurrently as a code owner July 9, 2025 17:19
kumaraditya303
kumaraditya303 reviewed Jul 11, 2025
View reviewed changes
@yoney
Copy link
Contributor Author

yoney commented Jul 14, 2025

@kumaraditya303 Thank you for the review!

@mpage mpage merged commit 9363703 into python:main Jul 14, 2025
45 checks passed
@miss-islington-app
Copy link

miss-islington-app bot commented Jul 14, 2025

Thanks @yoney for the PR, and @mpage for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 14, 2025
@yoney @kumaraditya303 @miss-islington
pythongh-116738: Make grp module thread-safe (pythonGH-135434)
6e39a40 
Make grp module methods getgrgid() and getgrnam() thread-safe when the GIL is disabled and getgrgid_r()/getgrnam_r() C APIs are not available.
---------
(cherry picked from commit 9363703)

Co-authored-by: Alper <alperyoney@fb.com>
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
@bedevere-app
Copy link

bedevere-app bot commented Jul 14, 2025

GH-136658 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jul 14, 2025
kumaraditya303 added a commit that referenced this pull request Jul 15, 2025
@miss-islington @yoney @kumaraditya303
[3.14] gh-116738: Make grp module thread-safe (GH-135434) (#136658)
55eaaab 
gh-116738: Make grp module thread-safe (GH-135434)

Make grp module methods getgrgid() and getgrnam() thread-safe when the GIL is disabled and getgrgid_r()/getgrnam_r() C APIs are not available.
---------
(cherry picked from commit 9363703)

Co-authored-by: Alper <alperyoney@fb.com>
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
@yoney yoney deleted the ft_grpmodule_2 branch July 15, 2025 17:26
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@yoney @kumaraditya303 @taegyunkim
pythongh-116738: Make grp module thread-safe (python#135434)
c698b8d 
Make grp module methods getgrgid() and getgrnam() thread-safe when the GIL is disabled and getgrgid_r()/getgrnam_r() C APIs are not available.
---------

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@yoney @kumaraditya303 @Agent-Hellboy
pythongh-116738: Make grp module thread-safe (python#135434)
33afbe0 
Make grp module methods getgrgid() and getgrnam() thread-safe when the GIL is disabled and getgrgid_r()/getgrnam_r() C APIs are not available.
---------

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
kumaraditya303 added a commit to miss-islington/cpython that referenced this pull request Sep 9, 2025
@miss-islington @yoney @kumaraditya303
[3.14] pythongh-116738: Make grp module thread-safe (pythonGH-135434) (…
de7e2f7 
…python#136658)

pythongh-116738: Make grp module thread-safe (pythonGH-135434)

Make grp module methods getgrgid() and getgrnam() thread-safe when the GIL is disabled and getgrgid_r()/getgrnam_r() C APIs are not available.
---------
(cherry picked from commit 9363703)

Co-authored-by: Alper <alperyoney@fb.com>
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colesbury colesbury colesbury left review comments

@ZeroIntensity ZeroIntensity ZeroIntensity left review comments

@mpage mpage mpage approved these changes

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes

@Yhg1s Yhg1s Awaiting requested review from Yhg1s

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

Assignees

No one assigned

Labels

topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@yoney @bedevere-bot @mpage @colesbury @ZeroIntensity @kumaraditya303